Reject multi-qubit noise channels with clear error message#1020
Reject multi-qubit noise channels with clear error message#1020ChungusLord123 wants to merge 1 commit intotensorflow:masterfrom
Conversation
TFQ only supports single-qubit noise channels, but there was no
validation to catch multi-qubit channels like
cirq.asymmetric_depolarize(error_probabilities={'XX': 0.1}).
This led to cryptic errors downstream in the C++ parser, which
silently ignored the second qubit.
Added an early validation check in the serializer that rejects
multi-qubit noise channels with a clear error message explaining
the limitation and suggesting .on_each() as a workaround. All 8
channel serializers are now guarded, along with a corresponding test.
P.S. I'm a high school student interested in quantum computing —
happy to discuss further or iterate on the patch if helpful.
Fixes tensorflow#686
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of cryptic errors from multi-qubit noise channels by introducing an early validation check in the serializer. The implementation adds a check to all 8 noise channel serializers, raising a clear ValueError for unsupported multi-qubit channels. The accompanying test verifies the new behavior. The changes are well-implemented and solve the problem described. I have one suggestion to enhance the test coverage by including the specific channel type that motivated this fix.
| def test_serialize_multi_qubit_noise_channel(self): | ||
| """Ensure multi-qubit noise channels are rejected with clear error.""" | ||
| q0 = cirq.GridQubit(0, 0) | ||
| q1 = cirq.GridQubit(0, 1) | ||
| multi_qubit_circuit = cirq.Circuit( | ||
| cirq.DepolarizingChannel(p=0.1, n_qubits=2)(q0, q1)) | ||
| with self.assertRaisesRegex(ValueError, | ||
| "Multi-qubit noise channels"): | ||
| serializer.serialize_circuit(multi_qubit_circuit) |
There was a problem hiding this comment.
Great job on adding a test for this new validation! To make it more robust, I suggest expanding it to also test cirq.asymmetric_depolarize, which was the motivating example from the issue description. This will ensure that channels defined by error_probabilities with multi-qubit Pauli strings are also correctly rejected.
Additionally, the n_qubits argument in cirq.DepolarizingChannel is deprecated. It's better to omit it and let cirq infer the number of qubits from the on() call.
| def test_serialize_multi_qubit_noise_channel(self): | |
| """Ensure multi-qubit noise channels are rejected with clear error.""" | |
| q0 = cirq.GridQubit(0, 0) | |
| q1 = cirq.GridQubit(0, 1) | |
| multi_qubit_circuit = cirq.Circuit( | |
| cirq.DepolarizingChannel(p=0.1, n_qubits=2)(q0, q1)) | |
| with self.assertRaisesRegex(ValueError, | |
| "Multi-qubit noise channels"): | |
| serializer.serialize_circuit(multi_qubit_circuit) | |
| def test_serialize_multi_qubit_noise_channel(self): | |
| """Ensure multi-qubit noise channels are rejected with clear error.""" | |
| q0 = cirq.GridQubit(0, 0) | |
| q1 = cirq.GridQubit(0, 1) | |
| depol_circuit = cirq.Circuit( | |
| cirq.DepolarizingChannel(p=0.1)(q0, q1)) | |
| with self.assertRaisesRegex(ValueError, "Multi-qubit noise channels"): | |
| serializer.serialize_circuit(depol_circuit) | |
| asym_depol_circuit = cirq.Circuit( | |
| cirq.asymmetric_depolarize(error_probabilities={'XX': 0.1})(q0, q1)) | |
| with self.assertRaisesRegex(ValueError, "Multi-qubit noise channels"): | |
| serializer.serialize_circuit(asym_depol_circuit) |
1 participant
Summary
cirq.asymmetric_depolarize(error_probabilities={'XX': 0.1}). This led to cryptic errors downstream in the C++ parser, which silently ignored the second qubit..on_each()as a workaround.Fixes #686
Test plan
test_serialize_multi_qubit_noise_channeltest verifies multi-qubit channels are rejected with a clear errorP.S. I'm a high school student interested in quantum computing — happy to discuss further or iterate on the patch if helpful.